Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 19, 2025

Summary

What changed?

  • Introduced more specific logging categories for gRPC worker logs: Microsoft.DurableTask.Worker.Grpc (previously Microsoft.DurableTask)
  • Added DualCategoryLogger class that emits logs to both new specific categories and legacy Microsoft.DurableTask when UseLegacyCategories is true
  • Added DurableTaskWorkerOptions.Logging.UseLegacyCategories configuration option (default: true) for backward compatibility
  • Updated IntegrationTestBase.GetLogs() to use the specific category to avoid duplicate matching when dual-category logging is enabled
  • Added comprehensive unit tests for the dual-category logging feature
  • Improved exception handling in CompositeDisposable.Dispose() to ensure both disposables are cleaned up even if one throws
  • Improved integration tests to verify the actual worker logger type using reflection with extracted helper methods

Why is this change needed?

The current logger category assignments are overly broad. Logs related to work item processing and gRPC are under the generic Microsoft.DurableTask category, making them hard to filter. This change introduces more specific categories while maintaining backward compatibility through dual-emission logging.

Issues / work items


Project checklist

  • Release notes are not required for the next release
  • Backport is not required
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change?
    • Setting UseLegacyCategories = false stops emitting to Microsoft.DurableTask category
    • Log filters expecting this category will need updates
    • Default behavior (UseLegacyCategories = true) is non-breaking

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot Agent
  • AI-assisted areas/files:
    • src/Worker/Core/DurableTaskWorkerOptions.cs (LoggingOptions class)
    • src/Worker/Grpc/DualCategoryLogger.cs (new file)
    • src/Worker/Grpc/GrpcDurableTaskWorker.cs (CreateLogger method)
    • test/Grpc.IntegrationTests/IntegrationTestBase.cs
    • test/Worker/Grpc.Tests/LoggingCategoryTests.cs (new file)
    • test/Worker/Grpc.Tests/RunBackgroundTaskLoggingTests.cs
  • What you changed after AI output: Reviewed and verified the implementation, fixed exception handling in CompositeDisposable.Dispose, improved tests with reflection and helper methods, consolidated documentation

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed (117 integration tests, 43 Worker.Grpc.Tests including 8 logging category tests)

Manual validation (only if runtime/behavior changed)

  • N/A (logging infrastructure change, behavior validated through automated tests)

Notes for reviewers

  • Migration Path:
services.AddDurableTaskWorker(options =>
{
    // Step 1: Update your log filters to use new categories
    // Step 2: Test with both categories emitting (default)
    // Step 3: Disable legacy when ready
    options.Logging.UseLegacyCategories = false;
});
  • The DualCategoryLogger ensures backward compatibility by emitting to both the new specific category and the legacy Microsoft.DurableTask category when UseLegacyCategories is true (default)
  • Test infrastructure was updated to use the specific new category to avoid duplicate log matching
  • Integration tests use reflection with helper methods (CreateWorkerWithLoggingOptions and GetWorkerLogger) to verify the actual worker logger type
Original prompt

investigate test failure in https://github.com/microsoft/durabletask-dotnet/actions/runs/20376967944/job/58557949619?pr=551 and fix


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Investigate and fix test failure in DurableTask Fix dual-category logging causing duplicate log entries in tests Dec 19, 2025
Copilot AI requested a review from YunchuWang December 19, 2025 18:43
@YunchuWang YunchuWang marked this pull request as ready for review December 19, 2025 19:08
Copilot AI review requested due to automatic review settings December 19, 2025 19:08
@YunchuWang YunchuWang changed the title Fix dual-category logging causing duplicate log entries in tests Add specific logging categories for Worker.Grpc and orchestration logs with backward-compatible opt-in Dec 19, 2025
@YunchuWang
Copy link
Member

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

Copy link
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

Updated PR description to follow .github/PULL_REQUEST_TEMPLATE.md format in commit 2429a1f.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a test failure where dual-category logging (introduced in PR #551) caused duplicate log entries. The issue occurred because TestLogProvider.TryGetLogs() uses StartsWith matching, so querying for Microsoft.DurableTask matched both the legacy category and the new Microsoft.DurableTask.Worker.Grpc category, counting each log message twice.

Key Changes:

  • Updated test log queries to use the specific Microsoft.DurableTask.Worker.Grpc category instead of the legacy prefix to avoid duplicate matching
  • Added comprehensive unit tests for the dual-category logging feature
  • Introduced logging configuration options to control legacy category emission

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/Worker/Grpc/DualCategoryLogger.cs New internal logger wrapper that emits logs to both primary and legacy categories for backward compatibility
src/Worker/Grpc/GrpcDurableTaskWorker.cs Updated to use the new CreateLogger method that creates a dual-category logger when legacy categories are enabled
src/Worker/Core/DurableTaskWorkerOptions.cs Added LoggingOptions class with UseLegacyCategories property to control dual-category logging behavior (defaults to true for backward compatibility)
test/Worker/Grpc.Tests/LoggingCategoryTests.cs New comprehensive test suite for dual-category logging functionality with helper classes for test infrastructure
test/Worker/Grpc.Tests/RunBackgroundTaskLoggingTests.cs Updated category constant to use the specific gRPC category to avoid duplicate log matching
test/Grpc.IntegrationTests/IntegrationTestBase.cs Updated GetLogs() method to query the specific Microsoft.DurableTask.Worker.Grpc category with detailed comments explaining the fix

@YunchuWang
Copy link
Member

@copilot no this pr changes include all the changes and fix both, update description to reflect actual changes, it is not just a fix pr

@YunchuWang
Copy link
Member

@copilot address all comments

@YunchuWang YunchuWang changed the title Add specific logging categories for Worker.Grpc and orchestration logs with backward-compatible opt-in Add specific logging categories for Worker.Grpc with backward-compatible opt-in Dec 19, 2025
Copy link
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot no this pr changes include all the changes and fix both, update description to reflect actual changes, it is not just a fix pr

Updated PR description to reflect the full dual-category logging feature including:

  • New specific logging categories (Microsoft.DurableTask.Worker.Grpc)
  • DualCategoryLogger for backward-compatible dual-emission
  • LoggingOptions.UseLegacyCategories configuration
  • Migration path for users

See commit b2b04ee.

Copilot AI changed the title Add specific logging categories for Worker.Grpc with backward-compatible opt-in Add specific logging categories for Worker.Grpc and orchestration logs with backward-compatible opt-in Dec 19, 2025
Comment on lines +95 to +98
catch (Exception ex)
{
firstException = ex;
}
Copy link
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot address all comments

Addressed all comments in commit dadaee9:

  1. Exception handling in Dispose: Fixed in b2b04ee - now handles exceptions from either disposable using try-catch and AggregateException
  2. this. for class members: Verified that lines 264, 266, and 297 already use this. correctly - these were false positives from the automated reviewer
  3. Test improvements: Improved the integration tests to actually verify the worker's logger type using reflection, rather than testing manually created loggers

Copilot AI requested a review from cgillum December 19, 2025 21:45
@YunchuWang YunchuWang merged commit 34e8e9b into main Dec 20, 2025
8 checks passed
@YunchuWang YunchuWang deleted the copilot/investigate-test-failure-again branch December 20, 2025 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make logging categories more specific

4 participants